Codecov Report

Merging #2711 (3c24761) into main (1ba336e) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 3c24761 differs from pull request most recent head 1f4a8cb. Consider uploading reports for the commit 1f4a8cb to get more accurate results

@@           Coverage Diff           @@
##             main    #2711   +/-   ##
=======================================
  Coverage   65.74%   65.75%           
=======================================
  Files         233      233           
  Lines       17844    17846    +2     
=======================================
+ Hits        11732    11734    +2     
  Misses       6112     6112           

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@keflavich - We've briefly discussed this, and would prefer to have one layer flatter namespace for the different irsa services, e.g. have astroquery.ipac.irsa.Irsa to handle the generic services based on tap, sia, etc, but have the one-off ones in this namespace, too, e.g. astroquery.ipac.irsa.Most and astroquery.ipac.irsa.Sofia I know that ibe and sha currently is one layer down, but those are legacy services that got moved unto astroquery.ipac.irsa from the main namespace.

Right, I forgot about that. Moved.

Oh, what I really meant is to not have a sofia submodule, but have the class directly in the irsa namespace (e.g. preferably in a sofia.py file, instead of core.py, etc.), similarly how mast has multiple classes in the mast namespace.

The only possible complication I see is the shared default config.

(also, something went wrong with the move, and now you've deleted all the files rather than moved them)

🤦 added it back.

But it might be a good idea to keep this one separate within this space because the implementation is so different? I'm ok either way; I'm not sure when I'm going to finish this though.

My thinking is these are all IRSA services, and the added namespace doesn't really adds much. I would not suggest to add support for any of the mission specific services in the Irsa() class, but to not have an additional layer such as astroquery.ipac.irsa.sofia.Sofia(), but expose it directly in astroquery.ipac.irsa.Sofia()

As for docs, having a separate sofia.rst would be nice, and I'll suggest chopping up mast.rst, too.

Ah, ok, I can lower the namespace a level. I think it's helpful to have sofia.py though.

and yes, sofia.rst for docs sounds good

I think it's helpful to have sofia.py though.

Yeap, each service class should have its own file. I'll do an audit on sha and ibe, too, if they will still be needed after we switch to the VO backends, I think they should also be exposed in the irsa namespace.

@keflavich - FYI: SOFIA datasets are getting moved to be served by the VO backends, so they will be available using the generic irsa module (at least the images will be available, we don't yet have an SSA layer, but if needed that can be done and is preferred as opposed to a custom sofia only tool). I would suggest this module won't be necessary and the PR should eventually be closed without merging.

SOFIA is landed in the VO backends, so I'm closing this now.

Please try and play with it, and add examples to the IRSA docs (or if there are too many of them, we can break it out to a separate page, but nevertheless using TAP and SIA you should be able to access most of the data (and I'm actively adding SSA methods, so the spectral examples will be covered very soon, too.).